netbsd/riscv64.rs: make changes so that this builds again. by he32 · Pull Request #4886 · rust-lang/libc

he32 · 2025-12-19T17:13:47Z

Description

These are changes which appear to be required to get this file to build (and cross-build) again.
Admittedly, some of the additions are based on error messages emitted by the rustc compiler.

Sources

Checklist

Relevant tests in libc-test/semver have been updated
No placeholder or unstable values like *LAST or *MAX are
included (see #3131)
Tested locally (cd libc-test && cargo test --target mytarget);
especially relevant for platforms that may not be checked in CI

...to something which also passes the upstream automated checks. Pull request submitted at rust-lang/libc#4886

he32 · 2025-12-27T00:00:02Z

Not sure how those two failing CI jobs can be related to my proposed changes...

…finition. This is already part of pull request rust-lang#4886 which is yet to be applied.

he32 · 2025-12-29T17:46:36Z

Not sure how those two failing CI jobs can be related to my proposed changes...

Evidently caused by what I had not done (merged upstream 'main' branch into mine).

tgross35

Sorry for the breakage, thank you for the PR!

src/unix/bsd/netbsdlike/netbsd/riscv64.rs

tgross35 · 2026-01-03T01:32:55Z

src/unix/bsd/netbsdlike/netbsd/riscv64.rs

-s_no_extra_traits! {
-    pub union __fpreg {
-        pub u_u64: u64,
-        pub u_d: c_double,
+impl core::cmp::PartialEq for __fpreg {
+    fn eq(&self, other: &__fpreg) -> bool {
+        unsafe { self.u_u64 == other.u_u64 || self.u_d == other.u_d }
    }
 }
-
-cfg_if! {
-    if #[cfg(feature = "extra_traits")] {
-        impl PartialEq for __fpreg {
-            fn eq(&self, other: &Self) -> bool {
-                unsafe { self.u_u64 == other.u_u64 }
-            }
+impl core::cmp::Eq for __fpreg {}
+impl core::marker::Copy for __fpreg {}
+impl core::clone::Clone for __fpreg {
+    fn clone(&self) -> __fpreg {
+        *self
+    }
+}
+impl hash::Hash for __fpreg {
+    fn hash<H: hash::Hasher>(&self, state: &mut H) {
+        unsafe {
+            self.u_u64.hash(state);
        }
-        impl Eq for __fpreg {}
-        impl hash::Hash for __fpreg {
-            fn hash<H: hash::Hasher>(&self, state: &mut H) {
-                unsafe { self.u_u64.hash(state) };
-            }
+    }
+}
+impl core::fmt::Debug for __fpreg {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        unsafe {
+            f.debug_struct("__fpreg")
+                .field("u_u64", &self.u_u64)
+                .field("u_d", &self.u_d)
+                .finish()


Could you add back the s_no_extra_traits and cfg wrappers for __fpreg? You can just make it match

libc/src/unix/linux_like/linux/gnu/b64/s390x.rs

Lines 218 to 241 in 0a5a8b6

s_no_extra_traits! {

pub union fpreg_t {

pub d: c_double,

pub f: c_float,

}

}

cfg_if! {

if #[cfg(feature = "extra_traits")] {

impl PartialEq for fpreg_t {

fn eq(&self, _other: &fpreg_t) -> bool {

unimplemented!("traits")

}

}

impl Eq for fpreg_t {}

impl hash::Hash for fpreg_t {

fn hash<H: hash::Hasher>(&self, _state: &mut H) {

unimplemented!("traits")

}

}

}

}

.

tgross35 · 2026-01-03T01:35:38Z

src/unix/bsd/netbsdlike/netbsd/riscv64.rs

-pub(crate) const _ALIGNBYTES: usize = size_of::<c_long>() - 1;
+pub(crate) const _ALIGNBYTES: usize = 0xf;


Do you have a source for this? Per https://github.com/NetBSD/src/blob/29216014b5d25ec3b457bbca2bb2bc6a80d7a1fd/sys/arch/riscv/include/cdefs.h#L6 it looks like the existing definition is correct.

(If you could add source links to the PR description for all changes here, that would be great)

I could still use some context here

rustbot · 2026-01-03T01:36:12Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

tgross35 · 2026-01-04T23:12:05Z

src/unix/bsd/netbsdlike/netbsd/riscv64.rs

    if #[cfg(feature = "extra_traits")] {
-        impl PartialEq for __fpreg {
-            fn eq(&self, other: &Self) -> bool {
-                unsafe { self.u_u64 == other.u_u64 }
+        impl core::cmp::PartialEq for __fpreg {
+            fn eq(&self, other: &__fpreg) -> bool {
+                unsafe { self.u_u64 == other.u_u64 || self.u_d == other.u_d }
+            }
+        }
+        impl core::cmp::Eq for __fpreg {}
+        impl core::marker::Copy for __fpreg {}
+        impl core::clone::Clone for __fpreg {
+            fn clone(&self) -> __fpreg {
+                *self
            }
        }
-        impl Eq for __fpreg {}
        impl hash::Hash for __fpreg {
            fn hash<H: hash::Hasher>(&self, state: &mut H) {
-                unsafe { self.u_u64.hash(state) };
+                unsafe {
+                    self.u_u64.hash(state);
+                }
+            }
+        }
+        impl core::fmt::Debug for __fpreg {
+            fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+                unsafe {
+                    f.debug_struct("__fpreg")
+                        .field("u_u64", &self.u_u64)
+                        .field("u_d", &self.u_d)
+                        .finish()
+                }
            }
        }


You should verify this builds with --features extra_traits. s_no_extra_traits adds a Clone, Copy, and Debug impl so I suspect it will fail.

tgross35 · 2026-01-04T23:12:36Z

src/unix/bsd/netbsdlike/netbsd/riscv64.rs

+        impl core::cmp::PartialEq for __fpreg {
+            fn eq(&self, other: &__fpreg) -> bool {
+                unsafe { self.u_u64 == other.u_u64 || self.u_d == other.u_d }
+            }
+        }
+        impl core::cmp::Eq for __fpreg {}
+        impl core::marker::Copy for __fpreg {}
+        impl core::clone::Clone for __fpreg {


No need to add the core::cmp prefix, it's in libc's prelude

tgross35 · 2026-01-04T23:17:39Z

src/unix/bsd/netbsdlike/netbsd/riscv64.rs

-        impl PartialEq for __fpreg {
-            fn eq(&self, other: &Self) -> bool {
-                unsafe { self.u_u64 == other.u_u64 }
+        impl core::cmp::PartialEq for __fpreg {
+            fn eq(&self, other: &__fpreg) -> bool {
+                unsafe { self.u_u64 == other.u_u64 || self.u_d == other.u_d }
+            }
+        }


Leave this body as-is, not having a false positive for ints is more important than indicating the -0==0 case for floats. Also matches other similar impls.

Or replace it with unimplemented! like the source I linked, these traits exist to fit in API signatures but they're going away in 1.0.

tgross35 · 2026-01-04T23:20:26Z

src/unix/bsd/netbsdlike/netbsd/riscv64.rs

-pub(crate) const _ALIGNBYTES: usize = size_of::<c_long>() - 1;
+pub(crate) const _ALIGNBYTES: usize = 0xf;


I could still use some context here

he32 · 2026-01-05T00:03:14Z

I could still use some context here

Oh? I thought I quoted from the gcc sources to adequatly address this. But to repeat in another way:

Gcc's riscv.h file (https://github.com/NetBSD/src/blob/trunk/external/gpl3/gcc/dist/gcc/config/riscv/riscv.h line 213)

/* There is no point aligning anything to a rounder boundary than this.  */
#define BIGGEST_ALIGNMENT 128

However, this is measured in bits, and gets converted to the predefined __BIGGEST_ALIGNMENT__ macro via
(https://github.com/NetBSD/src/blob/trunk/external/gpl3/gcc/dist/gcc/cppbuiltin.cc line 170):

  cpp_define_formatted (pfile, "__BIGGEST_ALIGNMENT__=%d",
			BIGGEST_ALIGNMENT / BITS_PER_UNIT);

which ends up as 16. This results in an __ALIGNBYTES value of 15 or 0xf via

#define __ALIGNBYTES    ((size_t)(__BIGGEST_ALIGNMENT__ - 1U))

(from /usr/include/machine/cdefs.h, visible via https://github.com/NetBSD/src/blob/trunk/sys/arch/riscv/include/cdefs.h)

tgross35 · 2026-01-05T00:37:27Z

Did you mean the comment in 7b39a6f? I did see that but a source link still helps in case there's nuance to be aware of. Makes sense though, that's the same source I linked but I incorrectly assumed __BIGGEST_ALIGNMENT__ was a byte value.

If you could instead change to

pub(crate) const __BIGGEST_ALIGNMENT__: usize = 128;
pub(crate) const _ALIGNBYTES: usize = __BIGGEST_ALIGNMENT__ - 1;

I think that would be best since it mirrors the source.

he32 · 2026-01-05T18:11:21Z

If you could instead change to

pub(crate) const __BIGGEST_ALIGNMENT__: usize = 128;
pub(crate) const _ALIGNBYTES: usize = __BIGGEST_ALIGNMENT__ - 1;

I think that would be best since it mirrors the source.

Actually, no, that is wrong. Internally in gcc, BIGGEST_ALIGNMENT is measured in bits. You need to divide by "bits-per-byte" before subtracting one to get the correct value for _ALIGNBYTES. Instead perhaps something like

// gcc for riscv64 defines `BIGGEST_ALIGNMENT`, but it's mesured in bits.
pub(crate) const __BIGGEST_ALIGNMENT_IN_BITS__: usize = 128;
// `_ALIGNBYTES` is measured in, well, bytes.
pub(crate) const _ALIGNBYTES: usize = (__BIGGEST_ALIGNMENT_IN_BITS__ / 8) - 1;

I assume I don't need to comment the literal 8...

tgross35

Forgot to mention the division but that's what I meant. Thanks for the updates, LGTM but could you please squash?

he32 · 2026-01-06T06:08:47Z

Forgot to mention the division but that's what I meant. Thanks for the updates, LGTM but could you please squash?

OK. I tried, but not only am I a rust newbie, I'm evidently also a git newbie, so what I thought would be one commit ended up as two for some reason or other. Do you want me to make another stab at squashing them back to one?

* Use the same type names as used by the native libc, to allow more self-tests to succeed * Remove un-needed imports, uncovered by libc's "cargo test" * Compute _ALIGNBYTES the same way gcc does. Verified by * Running (and passing) the libc self-tests "natively" on an emulated NetBSD/riscv64 system. * Cross-built rust 1.92.0 with this file in place for riscv64 in the vendored libc-0.2.17{5,6,7} versions, targeting NetBSD/riscv64. [ extracted this commit from the two in the PR - Trevor ]

rustbot · 2026-01-06T07:05:52Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

tgross35 · 2026-01-06T07:16:05Z

Forgot to mention the division but that's what I meant. Thanks for the updates, LGTM but could you please squash?

OK. I tried, but not only am I a rust newbie, I'm evidently also a git newbie, so what I thought would be one commit ended up as two for some reason or other. Do you want me to make another stab at squashing them back to one?

No worries, I took care of it :) it looks like you squashed the __pthread_spin_t commit with one of this PR's commits then squashed the rest , but the pthread bit was already applied as d4dd6d8.

For reference I just re-split the commits into pthread and the rest (git reset $(git merge-base HEAD main), then 2x git add -p followed by git commit to select hunks of the diff to add to a new commit) and then rebased. Probably also would have figured itself out with only a rebase, I just didn't realize what had happened at first.

Thanks for the patch!

* Use the same type names as used by the native libc, to allow more self-tests to succeed * Remove un-needed imports, uncovered by libc's "cargo test" * Compute _ALIGNBYTES the same way gcc does. Verified by * Running (and passing) the libc self-tests "natively" on an emulated NetBSD/riscv64 system. * Cross-built rust 1.92.0 with this file in place for riscv64 in the vendored libc-0.2.17{5,6,7} versions, targeting NetBSD/riscv64. [ extracted this commit from the two in the PR - Trevor ] (backport <rust-lang#4886>) (cherry picked from commit e9b8fa5)

* Use the same type names as used by the native libc, to allow more self-tests to succeed * Remove un-needed imports, uncovered by libc's "cargo test" * Compute _ALIGNBYTES the same way gcc does. Verified by * Running (and passing) the libc self-tests "natively" on an emulated NetBSD/riscv64 system. * Cross-built rust 1.92.0 with this file in place for riscv64 in the vendored libc-0.2.17{5,6,7} versions, targeting NetBSD/riscv64. [ extracted this commit from the two in the PR - Trevor ] (backport <#4886>) (cherry picked from commit e9b8fa5)

rustbot added O-riscv O-unix S-waiting-on-review labels Dec 19, 2025

he32 marked this pull request as draft December 19, 2025 18:44

rustbot removed the S-waiting-on-review label Dec 19, 2025

he32 marked this pull request as ready for review December 19, 2025 23:00

rustbot added the S-waiting-on-review label Dec 19, 2025

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Dec 19, 2025

rust192: transform the NetBSD riscv64 libc segment...

66deaf7

...to something which also passes the upstream automated checks. Pull request submitted at rust-lang/libc#4886

he32 marked this pull request as draft December 26, 2025 10:57

rustbot removed the S-waiting-on-review label Dec 26, 2025

he32 marked this pull request as ready for review December 26, 2025 23:59

rustbot added the S-waiting-on-review label Dec 26, 2025

This comment has been minimized.

Sign in to view

he32 added a commit to he32/libc that referenced this pull request Dec 29, 2025

netbsd/mod.rs: change definition of pthread_spin_t to allow arch rede…

29a420b

…finition. This is already part of pull request rust-lang#4886 which is yet to be applied.

tgross35 requested changes Jan 3, 2026

View reviewed changes

rustbot added S-waiting-on-author and removed S-waiting-on-review labels Jan 3, 2026

he32 requested a review from tgross35 January 4, 2026 15:23

tgross35 requested changes Jan 4, 2026

View reviewed changes

tgross35 approved these changes Jan 5, 2026

View reviewed changes

he32 force-pushed the netbsd-riscv64-fix branch from 42f0450 to ac428c7 Compare January 6, 2026 06:05

This comment has been minimized.

Sign in to view

tgross35 force-pushed the netbsd-riscv64-fix branch from ac428c7 to ef16e3e Compare January 6, 2026 07:04

tgross35 force-pushed the netbsd-riscv64-fix branch from ef16e3e to 8005136 Compare January 6, 2026 07:05

tgross35 enabled auto-merge January 6, 2026 07:06

tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Jan 6, 2026

tgross35 added this pull request to the merge queue Jan 6, 2026

Merged via the queue into rust-lang:main with commit e9b8fa5 Jan 6, 2026
91 of 99 checks passed

tgross35 mentioned this pull request Jan 8, 2026

[0.2] Backports #4919

Merged

tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Jan 8, 2026

	s_no_extra_traits! {
	pub union fpreg_t {
	pub d: c_double,
	pub f: c_float,
	}
	}

	cfg_if! {
	if #[cfg(feature = "extra_traits")] {
	impl PartialEq for fpreg_t {
	fn eq(&self, _other: &fpreg_t) -> bool {
	unimplemented!("traits")
	}
	}

	impl Eq for fpreg_t {}

	impl hash::Hash for fpreg_t {
	fn hash<H: hash::Hasher>(&self, _state: &mut H) {
	unimplemented!("traits")
	}
	}
	}
	}

		pub(crate) const _ALIGNBYTES: usize = size_of::<c_long>() - 1;
		pub(crate) const _ALIGNBYTES: usize = 0xf;

Conversation

he32 commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Sources

Checklist

Uh oh!

he32 commented Dec 27, 2025

Uh oh!

This comment has been minimized.

he32 commented Dec 29, 2025

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tgross35 Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

tgross35 Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

tgross35 Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jan 3, 2026

Uh oh!

tgross35 Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

tgross35 Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

tgross35 Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

tgross35 Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

he32 commented Jan 5, 2026

Uh oh!

tgross35 commented Jan 5, 2026

Uh oh!

he32 commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

he32 commented Jan 6, 2026

Uh oh!

rustbot commented Jan 6, 2026

Uh oh!

tgross35 commented Jan 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

he32 commented Dec 19, 2025 •

edited

Loading

he32 commented Jan 5, 2026 •

edited

Loading